Add Automatic Gain Control #621

UnknownSuperficialNight · 2024-09-26T19:48:13Z

Implemented automatic gain control (AGC)

I implemented a to_f32 conversion for the sample as it was missing. I'm not entirely certain if this is optimal, so I welcome feedback on the implementation.

This pull request is in relation to. Closes #620

Here is my main.rs and Cargo.toml i used to test while developing:

use rodio::source::Source;
use rodio::{Decoder, OutputStream, Sink};
use std::fs::File;
use std::io::BufReader;

fn main() {
    // Get a output stream handle to the default physical sound device
    let (_stream, stream_handle) = OutputStream::try_default().unwrap();
    // Create a sink
    let sink = Sink::try_new(&stream_handle).unwrap();

    // Load a sound from a file, using a path relative to Cargo.toml
    let file =
        BufReader::new(File::open(env!("CARGO_MANIFEST_DIR").to_owned() + "/OOTW.mp3").unwrap());
    // Decode that sound file into a source
    let source = Decoder::new(file).unwrap();

    // Amplify the source
    let amplified_source = source.automatic_gain_control(1.0, 0.3, 3.0);

    // Add the amplified source to the sink
    sink.append(amplified_source);

    // The sound plays in a separate thread.
    sink.sleep_until_end();
}

[workspace]
members = [".", "rodio"]

[package]
name = "test_area"
version = "0.1.0"
edition = "2021"

[dependencies]
rodio = { path = "rodio" }

Id suggest an attack_time of around 0.5 to 2.5 that seems to be a sweet spot for songs I tested.
~~Also, target_level I adjusted to be 0.9 just to make sure it does not clip the audio; it was clipping for me at 1.0.~~

Don't know where to write documentation or if I should be the one writing it. Idk, let me know; thanks :)

dvdsk · 2024-09-26T21:12:29Z

Since I do not feel fully qualified to review the algorithmic side of this, I am going to ask the wider rodio community for help in reviewing. I will also read up on AGC and see if I can find any problem with the implementation.

@iluvcapra you are working on synthesizers and those looked pretty complicated, would you be willing to give this a look too?

src/source/agc.rs

src/source/mod.rs

src/source/agc.rs

dvdsk

Very readable thanks! I have some minor things here and there. The algorithm makes sense to me though ill need to read up on AGC and look closer tomorrow. Did you come up with it yourself or did you find it somewhere? In either case we should attribute it in a comment somewhere.

Regarding the to_32. That seems like a fine solution to me. I am wondering though would the algorithm still work if you had only i16? We could implement subtract, add multiply_by_f32, div_by_usize and modulo for Sample. There are trait in the std for all those I think.

That might save some conversions to f32, and in doing so speed up the implementation.

…simplicity

UnknownSuperficialNight · 2024-09-26T22:46:27Z

I am wondering though would the algorithm still work if you had only i16? We could implement subtract, add multiply_by_f32, div_by_usize and modulo for Sample. There are trait in the std for all those I think.

Ill look into it, but probably would need other people's input on what would be best to do there. It could be a premature optimization, but if it works, then that's a bonus.

Thanks for the suggestion :)

Did you come up with it yourself or did you find it somewhere?

I put this together myself, but I definitely used some algorithms and ideas I found online. So while I wrote the code, I built on what others have done before.

By algorithms i mean mathematical ones

In either case we should attribute it in a comment somewhere.

Where?

src/source/agc.rs

- Implement asymmetric attack/release - Introduce MIN_ATTACK_TIME limit to prevent AGC instability - Clamp attack_time to prevent instability - Faster decrease, slower increase for smoother sound - Safeguard against extreme gain fluctuations

UnknownSuperficialNight · 2024-09-27T08:23:28Z

Integrated it with my GUI earlier, and it's working really well—Tested on a -10db audio file. Here’s a video showing the results:

(Left is using sink without AGC)
(Right is using sink with AGC)

Video:

2024-09-27.20-00-04.mp4

Let me know what you think of the new comment style and whether it is okay and descriptive enough.

dvdsk · 2024-09-27T13:45:20Z

It could be a premature optimization

you got a good point there. It might very well be, its a real bummer rodio has no benchmarks/profile runs setup. We should not optimize without benchmark or profile.

dvdsk · 2024-09-27T13:48:28Z

Where?

maybe the top of the source file? If you can list your inspiration and put your name on it, you made an algorithm for AGC you deserve some credit for that. Something along the likes of:

// Automatic gain control by @UnknownSuperficialNight, 
// this is a combination of <concept> other <concept> it builds upon 
// the work of <name> as seen here <blog/book/something>.

Its entirely optional if you do not feel like it you can leave this out.

dvdsk · 2024-09-27T13:51:05Z

Integrated it with my GUI earlier, and it's working really well—Tested on a -10db audio file. Here’s a video showing the results:

Great to hear! unfortunately the video is not playing for me (neither in browser or downloaded in VLC).

Edit: video now works 🎉

dvdsk

Honestly, this is the most well documented code I have seen in a long while ❤️. I am gonna show this to my friends. You have set a high bar for all the other PR's now :)

The only thing that could be improved is the --debug-gain flag, I think checking an argument as a library is sup-optimal.

Your comment there mentions its for both debugging and monitoring. Do you mean its useful for the end user to be able to see the number? For example so they can tweak the parameters better?

If that's true then maybe a callback would be a good idea. Automatic gain control could get an extra argument (maybe an option?).

It would look like this then:

    fn automatic_gain_control(
        self,
        target_level: f32,
        attack_time: f32,
        absolute_max_gain: f32,
        gain_monitor: Option<Box<dyn Fn(f32)>>,
    ) -> AutomaticGainControl<Self>

The current monitoring code in fn next could then become:

if let Some(callback) = self.gain_monitor {
    callback(self.current_gain);
}

I have used Box<dyn Fn> to prevent AGC getting a generic argument. That will incur a performance cost when using monitoring, its a tradeoff. What we could do is add a second source: MonitoredAutomaticGainControl which is generic over the monitor callback. Then implement the current AutomaticGainControl by just calling MonitoredAutomaticGainControl with as callback a function that does nothing. The optimizer will optimize that function out for us.

Regarding logging for debugging purposes. Its probably a good idea to add either tracing or log to Rodio. We can enable them using a compile time feature so they are zero cost for any user that does not need them. They also integrate nicely in whatever application someone is writing.

dvdsk · 2024-09-27T14:16:58Z

I should really add some benchmarking to Rodio. AGC is now running for every sample, might be an idea to make it run only once every n samples to lower the perf hit of AGC. But that is definitely not for now but for a future PR/issue once we have benchmarking integrated into Rodio's CI.

UnknownSuperficialNight · 2024-09-27T22:18:47Z

might be an idea to make it run only once every n samples to lower the perf hit of AGC

Running every n samples could work, but we risk issues with the audio not being able to adjust when needed. For example, a bass drop or a loud kick would possibly have a gain that is too high, making a crackling sound.

Instead, we can make a threshold that logs the previous values and detects a large enough change.

This is my testing while developing (AGC responsiveness threshold)

Performance results from testing based on cpu usage:
Every single thread (baseline):
1.4% to 4.2% of 1 core (avg 2.8%-4.2%)

Using a advanced detection method:
1.4% to 4.2% of 1 core (avg 2.8%)

Using updated and simplified advanced detection method:
1.4% to 4.2% of 1 core (same as above but no perceivable change in audio)

Although it most of the time indicates a 2.8% cpu usage, it's still such a small difference that I don't know if it's worth it. Plus, this is all using one core. By that, I mean 100% equals one core, and 200% equals two cores fully maxed out. In this case, we are using 1/36th of a core on average. So, maybe it's not worth it; the more delay we have, the more potential audio issues we will encounter, as it might not be able to react in time to lower it enough reasonably.

In any case here is the code for that if we want to explore it in the future

/// Checks if the peak level change is significant enough to trigger a gain update
///
/// This method calculates the absolute difference between the current peak level
/// and the last significant peak level. It considers a change to be significant
/// if this difference is 0.01 or greater, which is equivalent to a 1% change
/// in the signal level.
#[inline]
fn is_significant_change(&self) -> bool {
// Calculate the absolute difference directly using f32
let difference = (self.peak_level - self.last_significant_peak).abs();

// Consider a change significant if it's 0.01 or more (1% change)
 difference >= 0.01
}

UnknownSuperficialNight · 2024-09-27T23:07:15Z

I should really add some benchmarking to Rodio

definitely a good idea

Honestly, this is the most well documented code I have seen in a long while ❤️. I am gonna show this to my friends. You have set a high bar for all the other PR's now :)

Thank you so much! That really means a lot to me☺️

The only thing that could be improved is the --debug-gain flag, I think checking an argument as a library is sup-optimal.
Your comment there mentions its for both debugging and monitoring. Do you mean its useful for the end user to be able to see the number? For example so they can tweak the parameters better?
If that's true then maybe a callback would be a good idea. Automatic gain control could get an extra argument (maybe an option?).

Originally, it was just so I could view the changes while developing. However, now that I've refined the algorithm, it's become quite handy to read the values visually. This way, you don't have to guess using your ears. Realizing this, I'm planning to add a toggle for it, so we don't need to parse and loop over all arguments.

maybe the top of the source file? If you can list your inspiration and put your name on it, you made an algorithm for AGC you deserve some credit for that. Something along the likes of:

I don’t mind. ☺️ However, to clarify, my AGC algorithm is inspired by various online resources related to gain control concepts, including discussions on the RMS algorithm. While I didn't base it on a specific algorithm, I synthesized ideas from multiple sources to create my implementation.

If you're asking about the original inspiration for why I decided to work on this, it's because I am creating a graphical music player entirely in Rust, with the exception of two C backup dependencies. I wanted the audio to be automatically leveled and clear.

UnknownSuperficialNight · 2024-09-27T23:19:11Z

Force pushed so i don't add redundant commits to the history as i just merged the commits together

UnknownSuperficialNight · 2024-09-27T23:35:05Z

I have used Box to prevent AGC getting a generic argument. That will incur a performance cost when using monitoring, its a tradeoff. What we could do is add a second source: MonitoredAutomaticGainControl which is generic over the monitor callback. Then implement the current AutomaticGainControl by just calling MonitoredAutomaticGainControl with as callback a function that does nothing. The optimizer will optimize that function out for us.

This is a good idea but i wonder since our users will only do this once probably in debug mode why not just do

// Output current gain value for debugging purposes
#[cfg(debug_assertions)]
println!("Current gain: {}", self.current_gain);

The upside is that Rust's compiler's dead code elimination typically prevents unused functions and types from being included in the final binary of the application using this library. So, if people don't call automatic_gain_control, it shouldn't be compiled in. This allows us to benefit from the debug features without affecting other users or adding MonitoredAutomaticGainControl, which could increase the Rodio binary size. However, the downside is that if we do call automatic_gain_control, it will always compile the println in debug mode. What do you think?

dvdsk · 2024-09-28T00:06:12Z

Although it most of the time indicates a 2.8% cpu usage, it's still such a small difference that I don't know if it's worth it.

I completely agree, thats not worth the effort and increased code complexity 👍

dvdsk · 2024-09-28T00:16:13Z

However, the downside is that if we do call automatic_gain_control, it will always compile the println in debug mode. What do you think?

If anyone needs access to the value via a callback they can always open an issue, and then its easy enough to add. Always printing in debug mode seems like a bad idea, imagine having a tui app and in debug mode the view gets corrupted 😅. But the idea of determining compile time whether to print it is good.

I see two options there (maybe there are more):

add a feature to rodio, something like "print-agc-gain". Would still mess up tui-apps.
integrate tracing or log into rodio. They are what the rust community has standardised on. They can be disabled using a compile-time-feature, and allow the app developer to determine where the log messages go (stdout/file/some online log service). They can even be configured at runtime whether to display logs from a certain dependency.

It seems best to go for the second option, though that is out of the scope for this PR, ill see if I can quickly add tracing to rodio.

UnknownSuperficialNight · 2024-09-28T00:39:06Z

It seems best to go for the second option, though that is out of the scope for this PR, ill see if I can quickly add tracing to rodio.

Yea that seems best also look into env_logger it's integrated directly with log and allows control over logging through environment variables

Like this:
DEBUG_AUDIO=1 cargo run

Clarification I've never used any of these crates before, but I've heard of env_logger and the others.

dvdsk · 2024-09-28T00:57:33Z

Clarification I've never used any of these crates before, but I've heard of env_logger and the others.

tracing builds upon log and env_logger, its a bit heavier to compile but I think that is okay since you have to opt into it. These crates are always split into two parts: the core that only gives you the println equivalent and the part that uses those macro's to actually log somewhere. The app developer should use the latter and the libs the former.

I have added tracing on main now, see 95a466e on how to use it. And to see those traces in your tests/application you will want to do something like this: https://github.com/tokio-rs/tracing/blob/master/examples/examples/fmt.rs. For logging in AGC I would suggest tracing::trace("some msg/status").

UnknownSuperficialNight · 2024-10-01T16:06:10Z

Should be all done and ready to merge.

This is my second ever PR and my first ever code PR, so I'm happy it's gone well. 🙂

Btw, if you're working on remaking the sink API, I don't mind helping if you'd have me

benches/effects.rs

src/source/agc.rs

dvdsk

I would do the cfg experimental differently, and I think we still need an enable/disable function for the non atomicbool approach is missing. the rest looks perfect

dvdsk · 2024-10-02T11:55:50Z

This is my second ever PR and my first ever code PR, so I'm happy it's gone well. 🙂

and its a big one 600+ lines

Btw, if you're working on remaking the sink API, I don't mind helping if you'd have me

whats needed most is ideas for API design. I have not used rodio a ton myself (ironic isnt it). So I am not familiar with all usecases.

UnknownSuperficialNight · 2024-10-02T12:15:51Z

and its a big one 600+ lines

Don't know if you're saying that's a good or bad thing

whats needed most is ideas for API design. I have not used rodio a ton myself (ironic isnt it). So I am not familiar with all usecases.

I use it quite a lot in my music player GUI, in fact I've built around it

dvdsk · 2024-10-02T13:12:32Z

Don't know if you're saying that's a good or bad thing

well it means it was difficult/a lot of work most ppl start with a tiny PR :), you went big. So thats why it took a while.

UnknownSuperficialNight · 2024-10-02T14:28:45Z

well it means it was difficult/a lot of work most ppl start with a tiny PR

Well I learned a lot from it 😃

Waiting for another review when you have time (no pressure)

src/source/agc.rs

dvdsk

otherwise ready to merge

dvdsk · 2024-10-03T11:38:31Z

hype 👍, loving having this in gonna be great for my podcast app, once I write it :)

UnknownSuperficialNight · 2024-10-03T12:11:54Z

hype 👍, loving having this in gonna be great for my podcast app, once I write it :)

Yay, its finally merged 😌

Init commit for automatic_gain_control

85bfcbd

dvdsk reviewed Sep 26, 2024

View reviewed changes

src/source/agc.rs Show resolved Hide resolved

dvdsk reviewed Sep 26, 2024

View reviewed changes

src/source/agc.rs Outdated Show resolved Hide resolved

dvdsk reviewed Sep 26, 2024

View reviewed changes

src/source/mod.rs Show resolved Hide resolved

dvdsk reviewed Sep 26, 2024

View reviewed changes

src/source/agc.rs Outdated Show resolved Hide resolved

dvdsk requested changes Sep 26, 2024

View reviewed changes

Updated comments, refactored logic & added more member functions for …

625d0f2

…simplicity

UnknownSuperficialNight requested a review from dvdsk September 26, 2024 22:50

UnknownSuperficialNight commented Sep 26, 2024

View reviewed changes

src/source/agc.rs Outdated Show resolved Hide resolved

UnknownSuperficialNight commented Sep 26, 2024

View reviewed changes

src/source/agc.rs Outdated Show resolved Hide resolved

UnknownSuperficialNight added 2 commits September 27, 2024 13:04

Added simple flag to enable the debug temporarily during development

6b62544

dvdsk requested changes Sep 27, 2024

View reviewed changes

Add author credit to AGC implementation

97636d1

UnknownSuperficialNight force-pushed the feature/automatic-gain-control branch from e6c264e to 97636d1 Compare September 27, 2024 23:16

UnknownSuperficialNight added 4 commits October 2, 2024 04:31

Add experimental flag to enabled dynamic controls

3ce64ef

Merge branch 'master' into feature/automatic-gain-control

fd94703

Fix unused arc import

e2ee86e

Trigger CI checks

ef60286

UnknownSuperficialNight marked this pull request as ready for review October 1, 2024 15:50

UnknownSuperficialNight marked this pull request as draft October 1, 2024 15:51

UnknownSuperficialNight added 2 commits October 2, 2024 04:57

Fix agc_disable benchmark

af210a6

Add documentation to non experimental AutomaticGainControl

2610a27

UnknownSuperficialNight marked this pull request as ready for review October 1, 2024 16:06

UnknownSuperficialNight requested a review from dvdsk October 2, 2024 10:48

dvdsk reviewed Oct 2, 2024

View reviewed changes

benches/effects.rs Outdated Show resolved Hide resolved

dvdsk reviewed Oct 2, 2024

View reviewed changes

src/source/agc.rs Outdated Show resolved Hide resolved

dvdsk reviewed Oct 2, 2024

View reviewed changes

src/source/agc.rs Show resolved Hide resolved

dvdsk reviewed Oct 2, 2024

View reviewed changes

UnknownSuperficialNight added 2 commits October 3, 2024 01:06

Added getters

f8cf3c5

Added non-atomic is_enabled()

5ce1fff

UnknownSuperficialNight requested a review from dvdsk October 2, 2024 12:15

UnknownSuperficialNight marked this pull request as draft October 2, 2024 12:16

Remove experimental bench comment

bdbc159

UnknownSuperficialNight marked this pull request as ready for review October 2, 2024 12:20

dvdsk reviewed Oct 2, 2024

View reviewed changes

src/source/agc.rs Show resolved Hide resolved

dvdsk requested changes Oct 2, 2024

View reviewed changes

dvdsk merged commit c29fa1b into RustAudio:master Oct 3, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Automatic Gain Control #621

Add Automatic Gain Control #621

UnknownSuperficialNight commented Sep 26, 2024 •

edited

Loading

dvdsk commented Sep 26, 2024

dvdsk left a comment •

edited

Loading

UnknownSuperficialNight commented Sep 26, 2024 •

edited

Loading

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

dvdsk commented Sep 27, 2024

dvdsk commented Sep 27, 2024

dvdsk commented Sep 27, 2024 •

edited

Loading

dvdsk left a comment •

edited

Loading

dvdsk commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

UnknownSuperficialNight commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

dvdsk commented Sep 28, 2024

dvdsk commented Sep 28, 2024

UnknownSuperficialNight commented Sep 28, 2024 •

edited

Loading

dvdsk commented Sep 28, 2024

UnknownSuperficialNight commented Oct 1, 2024 •

edited

Loading

dvdsk left a comment

dvdsk commented Oct 2, 2024

UnknownSuperficialNight commented Oct 2, 2024

dvdsk commented Oct 2, 2024

UnknownSuperficialNight commented Oct 2, 2024 •

edited

Loading

dvdsk left a comment

dvdsk commented Oct 3, 2024

UnknownSuperficialNight commented Oct 3, 2024

Add Automatic Gain Control #621

Add Automatic Gain Control #621

Conversation

UnknownSuperficialNight commented Sep 26, 2024 • edited Loading

dvdsk commented Sep 26, 2024

dvdsk left a comment • edited Loading

Choose a reason for hiding this comment

UnknownSuperficialNight commented Sep 26, 2024 • edited Loading

UnknownSuperficialNight commented Sep 27, 2024 • edited Loading

dvdsk commented Sep 27, 2024

dvdsk commented Sep 27, 2024

dvdsk commented Sep 27, 2024 • edited Loading

dvdsk left a comment • edited Loading

Choose a reason for hiding this comment

dvdsk commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024 • edited Loading

UnknownSuperficialNight commented Sep 27, 2024

UnknownSuperficialNight commented Sep 27, 2024 • edited Loading

dvdsk commented Sep 28, 2024

dvdsk commented Sep 28, 2024

UnknownSuperficialNight commented Sep 28, 2024 • edited Loading

dvdsk commented Sep 28, 2024

UnknownSuperficialNight commented Oct 1, 2024 • edited Loading

dvdsk left a comment

Choose a reason for hiding this comment

dvdsk commented Oct 2, 2024

UnknownSuperficialNight commented Oct 2, 2024

dvdsk commented Oct 2, 2024

UnknownSuperficialNight commented Oct 2, 2024 • edited Loading

dvdsk left a comment

Choose a reason for hiding this comment

dvdsk commented Oct 3, 2024

UnknownSuperficialNight commented Oct 3, 2024

UnknownSuperficialNight commented Sep 26, 2024 •

edited

Loading

dvdsk left a comment •

edited

Loading

UnknownSuperficialNight commented Sep 26, 2024 •

edited

Loading

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

dvdsk commented Sep 27, 2024 •

edited

Loading

dvdsk left a comment •

edited

Loading

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

UnknownSuperficialNight commented Sep 27, 2024 •

edited

Loading

UnknownSuperficialNight commented Sep 28, 2024 •

edited

Loading

UnknownSuperficialNight commented Oct 1, 2024 •

edited

Loading

UnknownSuperficialNight commented Oct 2, 2024 •

edited

Loading